Skip to content

fix: System shared memory boundary check - #8335

Merged
yinggeh merged 3 commits into
mainfrom
yinggeh-DLIS-8400-shm-boundary-check-followup
Aug 9, 2025
Merged

fix: System shared memory boundary check#8335
yinggeh merged 3 commits into
mainfrom
yinggeh-DLIS-8400-shm-boundary-check-followup

Conversation

@yinggeh

@yinggeh yinggeh commented Aug 7, 2025

Copy link
Copy Markdown
Contributor

What does the PR do?

Followup from the original request. Similar bug in shared memory boundary checks.
Modify test cases to include register_offset in boundary checks.

Checklist

  • PR title reflects the change and is of format <commit_type>: <Title>
  • Changes are described in the pull request.
  • Related issues are referenced.
  • Populated github labels field
  • Added test plan and verified test passes.
  • Verified that the PR passes existing CI.
  • Verified copyright is correct on all changed files.
  • Added succinct git squash message before merging ref.
  • All template sections are filled out.
  • Optional: Additional screenshots for behavior/output changes with before/after.

Commit Type:

Check the conventional commit type
box here and add the label to the github PR.

  • fix

Related PRs:

#8334

Where should the reviewer start?

Test plan:

L0_shared_memory

  • CI Pipeline ID:
    32964778

Caveats:

Background

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

@yinggeh
yinggeh requested a review from tanmayv25 August 7, 2025 17:34
@yinggeh yinggeh self-assigned this Aug 7, 2025
@yinggeh yinggeh changed the title fix: System shared memory boundary check fix: System shared memory offset boundary check Aug 7, 2025
@yinggeh yinggeh changed the title fix: System shared memory offset boundary check fix: System shared memory boundary check Aug 7, 2025
@yinggeh yinggeh added the PR: fix A bug fix label Aug 7, 2025
create_byte_size=create_byte_size,
register_offset=register_offset,
)
offset = 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smaller offset for stricter check.

error_msg = []
self._configure_server()
create_byte_size = self.SYS_PAGE_SIZE + self.DEFAULT_SHM_BYTE_SIZE
register_offset = self.SYS_PAGE_SIZE

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this fix, out-of-bound would not be triggered if register_offset is non-zero.

size_t shm_region_size = 0;
if (it->second->byte_size_ > 0) {
shm_region_end += it->second->byte_size_ - 1;
shm_region_size += it->second->byte_size_;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove -1 for better readability.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This actually avoids a potential unsigned integer underflow if both offset and byte_size are 0. I had just encountered this underflow last week.

@yinggeh
yinggeh merged commit 50ff906 into main Aug 9, 2025
3 checks passed
@yinggeh
yinggeh deleted the yinggeh-DLIS-8400-shm-boundary-check-followup branch August 9, 2025 00:30
yinggeh added a commit that referenced this pull request Aug 9, 2025
mc-nv added a commit that referenced this pull request Aug 14, 2025
@kpedro88 kpedro88 mentioned this pull request Aug 19, 2025
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fix A bug fix

Development

Successfully merging this pull request may close these issues.

3 participants